Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

testament: fix missing error reporting #1451

Merged
merged 4 commits into from
Sep 8, 2024

Conversation

zerbina
Copy link
Collaborator

@zerbina zerbina commented Sep 7, 2024

Summary

Fix tests with invalid specs being silently ignored (they were neither
compiled nor run) when using all or cat megatest.

Details

The isJoinableSpec procedure didn't prevent tests with an invalid
spec (reInvalidSpec) from being joined, which overrode the failure
with reJoined. Since the specs then had no target enabled, they were
also not included in the megatest.

A few invalid test specifications went by unnoticed due to this bug --
they're now fixed.

If a test's specification is invalid, it must not be joined, so that an
error can be reported for it.

Previously, a test with an invalid spec could be joined, which
effectively discarded the error. The test was also neither run nor
compiled, since no targets were enabled for the test, in that case.
@zerbina zerbina added bug Something isn't working tool Improvements to non-compiler tooling labels Sep 7, 2024
@zerbina zerbina added this to the Tooling milestone Sep 7, 2024
@saem
Copy link
Collaborator

saem commented Sep 7, 2024

/merge

Copy link

github-actions bot commented Sep 7, 2024

Merge requested by: @saem

Contents after the first section break of the PR description has been removed and preserved below:


Notes for Reviewers

The line info of the assignment source expression didn't match the one
expected by the test.
@saem
Copy link
Collaborator

saem commented Sep 8, 2024

/merge

@chore-runner chore-runner bot added this pull request to the merge queue Sep 8, 2024
Merged via the queue into nim-works:devel with commit 677964c Sep 8, 2024
35 checks passed
@zerbina zerbina deleted the testament-fix-join-bug branch September 11, 2024 00:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working tool Improvements to non-compiler tooling
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants